Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: move util as a component #4504

Merged
merged 7 commits into from
Apr 12, 2019
Merged

Conversation

BusyJay
Copy link
Member

@BusyJay BusyJay commented Apr 10, 2019

What have you changed? (mandatory)

Extract util module from the TiKV sources to a standalone sub-project. It's part of process to speed up compilation by reducing code inside TiKV projects.

What are the type of the changes? (mandatory)

  • Engineering

How has this PR been tested? (mandatory)

unit tests

Does this PR affect documentation (docs) update? (mandatory)

No.

Does this PR affect tidb-ansible update? (mandatory)

No.

Refer to a related PR or issue link (optional)

#4165

Signed-off-by: Jay Lee <busyjaylee@gmail.com>
@BusyJay BusyJay added type/enhancement Type: Issue - Enhancement component/build-time Component: Compilation time labels Apr 10, 2019
@siddontang
Copy link
Contributor

how about the rebuild time now?

@BusyJay
Copy link
Member Author

BusyJay commented Apr 10, 2019

No notable changes detected. But building channel benchmark is much faster than before now.

@ngaut
Copy link
Member

ngaut commented Apr 10, 2019

Good job.

breezewish
breezewish previously approved these changes Apr 10, 2019
Copy link
Member

@breezewish breezewish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How long does it take to build channel benchmark now?

@breezewish
Copy link
Member

breezewish commented Apr 10, 2019

(related #3352)

@kennytm kennytm added the status/LGT1 Status: PR - There is already 1 approval label Apr 10, 2019
Copy link
Contributor

@brson brson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very straightforward refactor. Nice. I left one request to remove the tikv dep from test_util.

components/test_util/Cargo.toml Show resolved Hide resolved
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
siddontang
siddontang previously approved these changes Apr 11, 2019
Copy link
Contributor

@siddontang siddontang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

PTAL @brson

Signed-off-by: Jay Lee <busyjaylee@gmail.com>
brson
brson previously approved these changes Apr 11, 2019
Copy link
Contributor

@brson brson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brson brson mentioned this pull request Apr 11, 2019
@siddontang
Copy link
Contributor

please fix the conflicts @BusyJay

@kennytm kennytm added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels Apr 12, 2019
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
@siddontang
Copy link
Contributor

@kennytm @brson

Please approve it again.

@breezewish breezewish merged commit 8dae71a into tikv:master Apr 12, 2019
@BusyJay BusyJay deleted the util-as-component branch April 12, 2019 10:45
sticnarf pushed a commit to sticnarf/tikv that referenced this pull request Oct 27, 2019
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/build-time Component: Compilation time status/LGT2 Status: PR - There are already 2 approvals type/enhancement Type: Issue - Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants